Skip to content

Make code-review a dedicated category served by the BC PR Review engine - #774

Merged
Wenjie Fan (gggdttt) merged 21 commits into
mainfrom
bench/engine-review-runner
Aug 13, 2026
Merged

Make code-review a dedicated category served by the BC PR Review engine#774
Wenjie Fan (gggdttt) merged 21 commits into
mainfrom
bench/engine-review-runner

Conversation

@gggdttt

@gggdttt Wenjie Fan (gggdttt) commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

What & why

code-review in BC-Bench previously ran a bespoke /review prompt that the generic copilot / claude harnesses executed — a path that diverged from production (the BC-ALAgents review engine + BCQuality). This PR makes the BC PR Review engine the single implementation of code-review, and models it as a dedicated category command rather than a flavor of a harness, so the CLI/agent taxonomy stays clean.

Design

code-review is not a harness choice. The real AI harnesses are copilot / claude / bcal; code-review is a category that is always served by the fixed, Copilot-powered BC-ALAgents engine.

  • Dedicated command: run code-review / evaluate code-review run the engine's real generate half (Invoke-PRReviewShell.ps1 -GenerateOnly) in local mode, then score with the standard code-review judge. --model flows into the Copilot CLI the engine spawns, preserving model comparison.
  • No pseudo-harness: copilot / claude now reject --category code-review via a shared reject_code_review() helper that points to the dedicated command (previously copilot silently rerouted to the engine and claude errored with a cross-pointer).
  • Backend nested under copilot: the engine backend lives at agent/copilot/pr_review/ and is no longer exported as a top-level peer of run_copilot_agent / run_claude_code / run_bcal_agent. It is reached only through the code-review command.
  • Removed the now-unused code-review-template from agent/shared/config.yaml.

Custom BCQuality (config-driven, no MCP)

The engine fetches + filters BCQuality entirely through local PowerShell — no MCP dependency. Settings live in agent/shared/config.yaml under pr_review: (path + bcquality.{repo,ref,local_path}); CLI flags override:

  • --bcquality-local-path <dir> — copy + filter a local BCQuality checkout (never modifies it); the fast inner loop.
  • --bcquality-repo <owner/repo> + --bcquality-ref <ref> — a private branch/fork.
  • default — the engine's pinned repo/ref.

Provenance is recorded as BCQuality@<sha> (or local:<sha>) in the experiment configuration.

Workflows

  • copilot-evaluation.yml: split into a code-review engine step (if: category == 'code-review', runs bcbench evaluate code-review, checks out microsoft/BC-ALAgents@main, sets BC_PR_REVIEW_ROOT) and the existing Copilot step for all other categories. code-review results are summarized under the BC PR Review label.
  • claude-evaluation.yml: untouched (the claude command rejects code-review in code, so the category option needs no workflow edit).

Validation

  • ruff format --check, ruff check, ty check clean (only the pre-existing unrelated redteam.py warning).
  • pytest: 726 passed / 2 skipped.

AB#645219

Run the engine's own generate half (Invoke-PRReviewShell.ps1 -GenerateOnly) in local mode for the code-review category, so BC-Bench measures the real PROD generate path instead of a bespoke /review prompt. Maps the engine findings report (agent-output.txt) onto review.json for the existing scorer. Additive: copilot/claude/bcal agents are untouched.

AB#645219
Copilot AI balanced review requested due to automatic review settings August 6, 2026 12:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds BC-ALAgents as an opt-in code-review engine for BC-Bench.

Changes:

  • Adds engine execution and BCQuality preparation.
  • Maps engine findings into review.json.
  • Adds run/evaluate commands and mapping tests.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/bcbench/agent/__init__.py Exports the engine runner.
src/bcbench/agent/engine/__init__.py Defines the engine package API.
src/bcbench/agent/engine/agent.py Orchestrates engine execution.
src/bcbench/agent/engine/review_output.py Converts engine findings.
src/bcbench/agent/engine/scripts/Prepare-BCQualityRoot.ps1 Prepares filtered BCQuality content.
src/bcbench/agent/shared/config.yaml Adds engine configuration.
src/bcbench/commands/evaluate.py Adds engine evaluation command.
src/bcbench/commands/run.py Adds engine generation command.
tests/test_engine_review_output.py Tests finding conversion and parsing.
Suppressed comments (1)

src/bcbench/agent/engine/agent.py:177

  • The engine explicitly removes GH_TOKEN from the Copilot subprocess environment whenever REVIEW_SOURCE=local, so the token resolved below is never used. Headless/CI runs that rely on the documented GH_TOKEN prerequisite will fail authentication unless a separate Copilot credential store already exists. This needs an engine auth contract that forwards the token for local-diff evaluation, or a corresponding upstream engine change.
        "REVIEW_SOURCE": "local",

Comment thread src/bcbench/agent/engine/agent.py Outdated
Comment thread src/bcbench/agent/engine/agent.py Outdated
Comment thread src/bcbench/agent/engine/agent.py
Comment thread src/bcbench/agent/copilot/pr_review/review_output.py
Comment thread src/bcbench/agent/copilot/pr_review/agent.py
wenjiefan added 2 commits August 11, 2026 14:30
main tightened EvaluationContext.agent_name to AgentHarness and now reads .expected_metrics off it, so the engine runner's raw 'BC Review Engine' string was both a type error and a latent AttributeError at result-build time. Add a dedicated ENGINE harness (execution_time-only metrics, matching what run_engine_review reports) and use it in 'evaluate engine'.
@gggdttt
Wenjie Fan (gggdttt) marked this pull request as ready for review August 11, 2026 13:38
@gggdttt
Wenjie Fan (gggdttt) marked this pull request as draft August 12, 2026 07:39
wenjiefan and others added 3 commits August 12, 2026 10:01
Surface the engine's existing BCQUALITY_REPO/REF levers plus a new local-path mode through BC-Bench so BCQuality content can be evaluated without touching the engine.

- config.yaml: engine.bcquality.{repo,ref,local_path} (all null = engine default)

- Prepare-BCQualityRoot.ps1: -LocalPath copies (excl .git) + filters a local checkout, never mutating the source; provenance sha=local:<head>

- run/evaluate engine: --bcquality-repo / --bcquality-local-path (CLI overrides config)
Retire the bespoke /review prompt path that copilot/claude used: reject CODE_REVIEW in 'evaluate/run copilot|claude' (model comparison now flows through 'evaluate engine --model'), and remove the code-review-template from the shared prompt config.

De-pollute shared config: move the engine settings into an engine-owned agent/engine/config.yaml (read by _load_engine_settings), dropping the 'engine:' block from agent/shared/config.yaml.

Workflows: drop code-review from copilot/claude evaluation dispatch options and add engine-evaluation.yml as the sole code-review entry point, exposing engine-ref plus bcquality-ref/-repo overrides so BCQuality can be evaluated from a private branch.
@gggdttt Wenjie Fan (gggdttt) changed the title Add BC-ALAgents review engine as a BC-Bench code-review agent (AB#645219) Route code-review exclusively through the BC Review Engine Aug 12, 2026
wenjiefan added 14 commits August 12, 2026 12:40
Address review: drop the standalone engine-evaluation.yml. The existing code-review category stays in copilot-evaluation.yml; only the downstream code is replaced. 'evaluate/run copilot --category code-review' now delegate to the review engine via a shared helper (the --model threads into the Copilot the engine spawns); 'claude' rejects code-review with guidance since its model set is Claude-only.

The engine is checked out (microsoft/BC-ALAgents@main) only for the code-review category. Engine/BCQuality knobs (bcquality repo/ref/local_path, min_severity) live in agent/engine/config.yaml instead of manual workflow inputs, so testers evaluate a private BCQuality branch by editing config on their own branch.
…uched

Move the code-review engine knobs (path, agent_version, min_severity, bcquality) back into agent/shared/config.yaml under an 'engine:' block so they are easy to tweak alongside the other agent settings, and delete the separate engine-owned config.yaml. Restore claude-evaluation.yml to main: this PR no longer changes it (the 'claude' command still rejects code-review in code, so the category option stays as-is).
…null

CI supplies BC_REVIEW_ENGINE_ROOT (which takes precedence); a local run sets that env var or fills engine.path on a private branch.
agent_version and min_severity already have hard code defaults ('0.0.0' / 'Low') in run_engine_review, so listing them in config was redundant; drop them and keep only path and bcquality.{repo,ref,local_path}.
Drop the 'code-review always runs the engine' delegate comments (the branch condition + helper name already say it; model flow is documented in the engine agent docstring), the '2b integration' jargon in that docstring, and the redundant BC_REVIEW_ENGINE_ROOT inline note. Keep the comments that explain why (engine checkout, engine config knobs).
…ir, record engine revision

- _resolve_pwsh: drop the powershell (5.1) fallback so only PowerShell 7+ is used, matching the error message and the engine scripts' requirement.

- Prepare-BCQualityRoot.ps1: remove a reused fetch destination before init/fetch so a leftover root cannot carry stale (filtered-away) content or fail 'git remote add origin'.

- ExperimentConfiguration now records the resolved BC-ALAgents engine revision (with a dirty marker) as a bc-review-engine@<sha> plugin, so results from different engine commits are distinguishable.
…ng a clean review

_write_review_json now raises AgentError when agent-output.txt is empty or not a valid findings report (report is None), or when the report has no findings list (missing/non-list). Previously both silently produced an empty review.json, which the scorer credits as a correct 0-findings review (precision 1.0), hiding generation/dispatch failures. A legitimately empty review still carries a real findings: [] list and is preserved. Adds tests/test_engine_agent.py.
…_agent

The four run_* exports are distinct backends; run_engine_review spawns the PROD PowerShell orchestrator (Copilot lives inside the engine) and shares almost no parameters with run_copilot_agent. code-review is routed to the engine at the command layer, so it still lives under the copilot command for users. Adds notes at the __all__ export and the function docstring to pre-empt the recurring 'why not a copilot option?' question.
…5219)

The BC-Bench execution backend that runs the PROD BC-ALAgents review engine was named asymmetrically (run_engine_review) vs its three peers run_bcal_agent / run_claude_code / run_copilot_agent, which repeatedly drew reviewer questions about why it is a separate top-level backend.

Rename the backend org naming only: agent/engine -> agent/pr_review; run_engine_review -> run_pr_review_agent; _run_engine(_evaluation) helpers; CLI commands 'engine' -> 'pr-review'; AgentHarness.ENGINE member -> PR_REVIEW.

Deliberately keep the domain term 'engine' where it names the external PROD BC-ALAgents review engine: AgentHarness value stays 'BC Review Engine', BC_REVIEW_ENGINE_ROOT env, config engine.path, _resolve_engine_* helpers, engine_report_* functions, and copilot-evaluation.yml are untouched.
…ons block

When the dead code-review-template prompt was removed from config.yaml, the line immediately after it (the '# 1. whether to copy custom instructions' bullet of the instructions: control block) was deleted too, leaving an orphaned '# 2.' with no '# 1.'. Restore the bullet so the numbering matches the skills:/agents: blocks.
Align the remaining user-facing names with the pr_review backend:

- AgentHarness label value 'BC Review Engine' -> 'BC PR Review' (result agent_name + summarize dashboard); synced in copilot-evaluation.yml.

- Env var BC_REVIEW_ENGINE_ROOT -> BC_PR_REVIEW_ROOT (agent.py, workflow, config comment, docstrings).

- Config key engine.path -> pr_review.path (config block + accessors); helpers _load_engine_settings/_resolve_engine_root -> _load_pr_review_settings/_resolve_pr_review_root.

- Fixed stale 'bcbench evaluate engine' hint -> 'bcbench evaluate pr-review'.

Kept engine-provenance internals (bc-review-engine plugin, load_engine_report, product prose) as they name the external BC-ALAgents engine.
code-review is always served by the fixed BC-ALAgents review engine, so it no longer masquerades as a peer of the copilot/claude harnesses:

- Rename 'run/evaluate pr-review' -> 'run/evaluate code-review' (category-named, not harness-named).

- Remove the copilot code-review reroute (asking for copilot no longer silently runs the engine) and the claude cross-pointer error; both now reject code-review uniformly via a shared reject_code_review() helper that points to the dedicated command.

- CI: split into a code-review engine step and a Copilot step, so the workflow mirrors the command split instead of routing code-review through 'evaluate copilot --category'.
The PR-review engine backend is the Copilot-powered BC-ALAgents review engine, not a fourth AI harness. Move agent/pr_review -> agent/copilot/pr_review so the package tree reflects that, and drop run_pr_review_agent from the top-level bcbench.agent __all__ so it is no longer exported as a peer of run_copilot_agent/run_claude_code/run_bcal_agent. It is reached only via the dedicated code-review command, importing from bcbench.agent.copilot.pr_review.
@gggdttt Wenjie Fan (gggdttt) changed the title Route code-review exclusively through the BC Review Engine Make code-review a dedicated category served by the BC PR Review engine Aug 12, 2026
@gggdttt
Wenjie Fan (gggdttt) marked this pull request as ready for review August 13, 2026 14:40
@gggdttt
Wenjie Fan (gggdttt) enabled auto-merge (squash) August 13, 2026 14:46
auto-merge was automatically disabled August 13, 2026 14:50

Pull Request is not mergeable

@gggdttt
Wenjie Fan (gggdttt) enabled auto-merge (squash) August 13, 2026 14:57
auto-merge was automatically disabled August 13, 2026 15:01

Pull Request is not mergeable

@gggdttt
Wenjie Fan (gggdttt) enabled auto-merge (squash) August 13, 2026 15:15
auto-merge was automatically disabled August 13, 2026 15:20

Pull Request is not mergeable

@gggdttt
Wenjie Fan (gggdttt) enabled auto-merge (squash) August 13, 2026 15:31
@gggdttt
Wenjie Fan (gggdttt) merged commit 75c9d3f into main Aug 13, 2026
23 checks passed
@gggdttt
Wenjie Fan (gggdttt) deleted the bench/engine-review-runner branch August 13, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants